New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CBRD-25762] When a general user performs a trigger unload, remove [user_schema] from condition and action_definition if the owner is the user themselves. #5731

Open

jongmin-won wants to merge 1 commit into CUBRID:develop from jongmin-won:CBRD-25762

Contributor

jongmin-won commented Dec 21, 2024

http://jira.cubrid.org/browse/CBRD-25762

Purpose

기존 Trigger의 경우 name과 target_class에서 [user_schema]가 제거되지만, condition 및 action_definition에 사용된 쿼리 내 [user_schema]는 제거되지 않는 문제를 수정 합니다.

Repro

trigger 생성

create user u1;

create table u1.t1 (col1 int);
create table u1.t2 (col1 varchar);

CREATE TRIGGER u1.trigger
  STATUS ACTIVE
  PRIORITY 0.000000
  AFTER INSERT ON u1.t1
  IF 100 < (SELECT COUNT(*) FROM (SELECT /*+ NO_MERGE */ col1 as col1, 1 as col2 from u1.t2) as tbl WHERE tbl.col1 is not null)
  EXECUTE insert into u1.t2 select * from u1.t1;

unload 수행

cubrid unloaddb -u u1 -s testdb -S

Expected

cat testdb_trigger

CREATE TRIGGER [trigger]
  STATUS ACTIVE
  PRIORITY 0.000000
  AFTER INSERT ON [t1]
IF 100<(select count(*) from (select /*+ NO_MERGE */ [col1] as [col1], 1 as [col2] from [t2]) [tbl] where [tbl].[col1] is not null )
  EXECUTE insert into [t2] select * from [t1];

Actual

cat testdb_trigger

CREATE TRIGGER [trigger]
  STATUS ACTIVE
  PRIORITY 0.000000
  AFTER INSERT ON [t1]
IF 100<(select count(*) from (select /*+ NO_MERGE */ [col1] as [col1], 1 as [col2] from [u1.t2]) [tbl] where [tbl].[col1] is not null )
  EXECUTE insert into [u1.t2] select * from [u1.t1];

Implementation

N/A

Remarks

N/A


          When a general user performs a trigger unload, remove [user_schema] f…

be32f5e

…rom condition and action_definition if the owner is the user themselves.

jongmin-won self-assigned this

jongmin-won requested review from hgryoo, vimkim, hyunikn, kangmin5505 and ctshim

December 23, 2024 04:09

jongmin-won marked this pull request as ready for review

December 23, 2024 04:09

jongmin-won requested a review from beyondykk9 as a code owner

December 23, 2024 04:09

kangmin5505 reviewed

View reviewed changes

src/object/trigger_description.cpp

+              	      output_ctx ("/* ERROR : IF %s */\n", trigger->condition->source);
+              	      error = ER_OUT_OF_VIRTUAL_MEMORY;
+              	      er_set (ER_ERROR_SEVERITY, ARG_FILE_LINE, ER_OUT_OF_VIRTUAL_MEMORY, 1, (size_t) length);
+              	      free_and_init (text);

kangmin5505 Dec 23, 2024

Suggested change

free_and_init (text);

text == NULL 이기 때문에 free_and_init 을 하지 않아도 될 것 같습니다.

src/object/trigger_description.cpp

Comment on lines +342 to +346

+              	  parser = parser_create_parser ();
+              	  if (parser == NULL)
+              	    {
+              	      output_ctx ("/* ERROR : IF %s */\n", trigger->condition->source);
+              	    }

kangmin5505 Dec 23, 2024

아래 로직에서 parser 에 접근하기 때문에, parser == NULL 이면 함수를 종료해야 할 필요가 있어 보입니다.

src/object/trigger_description.cpp

+              	      query_condition_result = parser_print_tree_with_quotes (parser, *condition_node);
+              	      /* remove appended trigger evaluate info */
+              	      p = strstr (query_condition_result, remove_eval_prefix);

kangmin5505 Dec 23, 2024

p == NULL 을 처리해야 할 필요가 있어 보입니다.

src/object/trigger_description.cpp

Comment on lines +392 to +395

+              	      if (parser == NULL)
+              		{
+              		  output_ctx ("\n/* ERROR : EXECUTE %s */\n", trigger->action->source);
+              		}

kangmin5505 Dec 23, 2024

아래 로직에서 parser 에 접근하기 때문에, parser == NULL 이면 함수를 종료해야 할 필요가 있어 보입니다.

beyondykk9 reviewed

View reviewed changes

src/object/trigger_description.cpp

+              	      if (p != NULL)
+              		{
+              		  p = (char *) memmove (p, p + strlen (remove_eval_prefix), strlen (p) - strlen (remove_eval_prefix) + 1);
+              		}

Contributor

beyondykk9 Dec 23, 2024

else 구문으로 P가 NULL인 경우에 대한 처리가 필요합니다.

kangmin5505 reviewed

View reviewed changes

src/object/trigger_description.cpp

Comment on lines +322 to +323

		const char *eval_prefix = "EVALUATE ( ";
		const char *eval_suffix = " ) ";

kangmin5505 Dec 24, 2024

trigger_manager.c 에 EVAL_PREFIX 와 EVAL_SUFFIX 가 static const char 로 선언되어 있는데, 이것을 활용할 수 있도록 수정하는 것이 더 좋을 거 같습니다.

https://github.com/jongmin-won/cubrid/blob/be32f5eab076f699c955defe8db2da98a13462e9/src/object/trigger_manager.c#L128-L129

ctshim reviewed

View reviewed changes

src/object/trigger_description.cpp

Comment on lines +366 to +369

+              	      if (remove_eval_suffix_len > 0 && p[remove_eval_suffix_len - 1] == ')')
+              		{
+              		  p[remove_eval_suffix_len - 1] = '\0';
+              		}

Contributor

ctshim Dec 24, 2024

remove_eval_prefix로 시작했는데 끝에 ')'가 오지 않는 경우가 있을 수 있나요?
" ... ) " 와 같이 ')' 이후에 공백 문자가 포함되어 있을 가능성은 없나요?

ctshim reviewed

View reviewed changes

src/object/trigger_description.cpp

+              		  p = (char *) memmove (p, p + strlen (remove_eval_prefix), strlen (p) - strlen (remove_eval_prefix) + 1);
+              		}
+              	      remove_eval_suffix_len = strlen (p);

Contributor

ctshim Dec 24, 2024

362라인이 잘 수행된 후에는
remove_eval_suffix_len는 strlen (p) - strlen (remove_eval_prefix) 와 같아야겠지요?

ctshim approved these changes

View reviewed changes

ctshim mentioned this pull request

[CBRD-25719] Fix an issue where [user_schema] was incorrectly stored or missing in the condition and action_definition columns of the db_trigger catalog during trigger creation or loaddb execution with legacy unload files #5729

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kangmin5505 kangmin5505 left review comments

beyondykk9 beyondykk9 left review comments

ctshim ctshim approved these changes

hgryoo Awaiting requested review from hgryoo

vimkim Awaiting requested review from vimkim

hyunikn Awaiting requested review from hyunikn

Labels

None yet